Skip to content

feat: add prisma SQL migration for job-run internal flag backfill#564

Closed
hmacr wants to merge 2 commits into
triggerdotdev:mainfrom
hmacr:issue-557-job-run-internal-backfill
Closed

feat: add prisma SQL migration for job-run internal flag backfill#564
hmacr wants to merge 2 commits into
triggerdotdev:mainfrom
hmacr:issue-557-job-run-internal-backfill

Conversation

@hmacr

@hmacr hmacr commented Oct 5, 2023

Copy link
Copy Markdown
Contributor

Closes #557

This builds on top of #563 and should ideally be merged after #563 goes live.

The intention is to separate the schema and API change from the backfill script. This way, the jobs & job runs that get created during the backfill execution can still have the correct internal flags set (the API takes care of it).

✅ Checklist

  • I have followed every step in the contributing guide
  • The PR title follows the convention.
  • I ran and tested the code works

Testing

  • Verified the past runs of internal jobs have their job-run internal flag set to true.
  • Verified that the migration runs only once.

Changelog

  • Add a backfill script that sets the internal flag for past JobRun entries.
  • To avoid DB overload, chunks of jobs (100 - configurable) are used to load their associated job-runs.

💯

@changeset-bot

changeset-bot Bot commented Oct 5, 2023 •
edited
Loading

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 8dba22a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@hmacr
hmacr marked this pull request as draft October 5, 2023 09:06
@hmacr

hmacr commented Oct 5, 2023

Copy link
Copy Markdown
Contributor Author

Marking it as draft, since it has dependency on #563.
Needs to be rebased with the main branch once #563 is merged.

hmacr
hmacr commented Oct 5, 2023
View reviewed changes
Comment thread apps/webapp/prisma/migrations/jobRunInternal.ts Outdated
import chunk from "lodash.chunk";

const MIGRATION_NAME = "2023-10-05-SetJobRunInternalFlag";
const JOB_CHUNK_SIZE = 100;

@hmacr hmacr Oct 5, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should be the ideal chunk size?

@hmacr hmacr mentioned this pull request Oct 5, 2023
Closed
@ericallam

ericallam commented Oct 5, 2023

Copy link
Copy Markdown
Member

I like the idea of doing the backfilling separately, but I think we should try do the backfill in SQL using Prisma migrations. Something like:

UPDATE "JobRun"
SET "internal" = "Job"."internal"
FROM "Job"
WHERE "JobRun"."jobId" = "Job"."id";

@hmacr

hmacr commented Oct 5, 2023

Copy link
Copy Markdown
Contributor Author

Ah ok. I was worried that a large update would reduce the DB throughput for other processes, so wrote a backfill script.

I'll update the PR with the necessary SQL.

@hmacr
hmacr force-pushed the issue-557-job-run-internal-backfill branch from bb970f1 to 8dba22a Compare October 5, 2023 12:44
@hmacr hmacr changed the title feat: add backfill script for job-run internal flag feat: add prisma SQL migration for job-run internal flag backfill Oct 5, 2023
@hmacr

hmacr commented Oct 5, 2023

Copy link
Copy Markdown
Contributor Author

I've removed the backfill script and added the SQL to update the job-run internal flag.

The migration SQL worked as expected on local.

@ericallam ericallam mentioned this pull request Oct 6, 2023
Merged
3 tasks
@hmacr

hmacr commented Oct 6, 2023

Copy link
Copy Markdown
Contributor Author

Closing since this code is moved to #563

@hmacr hmacr closed this Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[TRI-1366] fix: JobRun needs an internal boolean column, to reflect the value in Job

2 participants

@hmacr @ericallam